kraken: allow cancelling install/update operations#3857
Open
nicoschmdt wants to merge 3 commits intobluerobotics:masterfrom
Open
kraken: allow cancelling install/update operations#3857nicoschmdt wants to merge 3 commits intobluerobotics:masterfrom
nicoschmdt wants to merge 3 commits intobluerobotics:masterfrom
Conversation
Reviewer's GuideImplements cancelable extension install/update workflows by wiring AbortController-based cancellation through the frontend, adding rollback via per-version uninstall APIs, improving streaming cleanup for aborted operations, and adjusting backend extension install/update semantics and status messaging. Sequence diagram for cancelable extension install/update workflowsequenceDiagram
actor User
participant ExtensionManagerView
participant PullProgress
participant KrakenManager
participant Axios
participant KrakenAPI as Kraken_API
participant Extension as Extension_backend
User->>ExtensionManagerView: click_install_or_update(extension, version)
ExtensionManagerView->>ExtensionManagerView: beginInstallOperation()
ExtensionManagerView->>PullProgress: show(cancelable=true)
ExtensionManagerView->>KrakenManager: installExtension/updateExtensionToVersion/finalizeExtension(..., signal)
KrakenManager->>Axios: back_axios(request_with_signal)
Axios->>KrakenAPI: HTTP_request(stream)
KrakenAPI->>Extension_backend: extension.install/update(...)
Extension_backend->>Extension_backend: _pull_docker_image()
Extension_backend-->>KrakenAPI: streaming_bytes(status_and_progress)
KrakenAPI-->>Axios: streaming_response
Axios-->>KrakenManager: progress_events
KrakenManager-->>ExtensionManagerView: progress_events
ExtensionManagerView-->>PullProgress: update_progress(download, extraction, statustext)
User->>PullProgress: click_Cancel
PullProgress-->>ExtensionManagerView: cancel_event
ExtensionManagerView->>ExtensionManagerView: cancelInstallOperation()
ExtensionManagerView->>Axios: AbortController.abort() via signal
Axios-->>KrakenAPI: connection_terminated
KrakenAPI->>Extension_backend: generator.aclose()
Extension_backend->>Extension_backend: unlock_and_cleanup()
Axios-->>KrakenManager: cancel_error(axios.isCancel)
KrakenManager-->>ExtensionManagerView: cancel_error
ExtensionManagerView->>KrakenManager: uninstallExtensionVersion(identifier, tag)
KrakenManager->>KrakenAPI: DELETE /extension/{identifier}/{tag}
KrakenAPI->>Extension_backend: uninstall()
Extension_backend-->>KrakenAPI: uninstall_complete
KrakenAPI-->>KrakenManager: 202 Accepted
KrakenManager-->>ExtensionManagerView: uninstall_done
ExtensionManagerView->>ExtensionManagerView: finishInstallOperation()
ExtensionManagerView->>PullProgress: hide(cancelable=false)
ExtensionManagerView->>ExtensionManagerView: fetchInstalledExtensions()
ExtensionManagerView-->>User: show_info_notification(install_or_update_cancelled)
Class diagram for updated extension install/update and frontend cancellation handlingclassDiagram
class Extension {
-identifier: str
-tag: str
-source: Any
-digest: Optional[str]
-unique_entry: str
+_create_extension_settings(should_enable: bool = True) ExtensionSettings
+_pull_docker_image(docker_auth: Optional[str]) AsyncGenerator[bytes, None]
+_clear_remaining_tags() AsyncGenerator[None, None]
+install(clear_remaining_tags: bool = True, should_enable: bool = True) AsyncGenerator[bytes, None]
+update(clear_remaining_tags: bool, should_enable: bool = True) AsyncGenerator[bytes, None]
+uninstall() None
+_disable_running_extension() Optional[Extension]
+_image_is_available_locally() bool
+lock(entry: str) None
+unlock(entry: str) None
+reset_start_attempt(entry: str) None
+_save_settings(settings: ExtensionSettings) None
+finalize_temporary_extension(temp_extension: Extension, identifier: str, body: Any) Extension
+from_manifest(identifier: str, tag: str) Extension
<<static>> _status_message(message: str) bytes
}
class ExtensionSettings {
+identifier: str
+name: str
+docker: str
+tag: str
+permissions: Any
+enabled: bool
+user_permissions: Any
}
Extension --> ExtensionSettings : creates
class ExtensionRouterV2 {
+install(body: ExtensionSource) StreamingResponse
+install_from_identifier(identifier: str, body: ExtensionInstall) StreamingResponse
+update_to_tag(identifier: str, tag: str, purge: bool = True, should_enable: bool = True) StreamingResponse
+finalize_extension(temp_identifier: str, body: ExtensionFinalize) StreamingResponse
}
ExtensionRouterV2 --> Extension : uses
class ExtensionRouterV1 {
+install_extension(body: ExtensionSource) StreamingResponse
}
ExtensionRouterV1 --> Extension : uses
class ExtensionManagerView {
-active_abort_controller: AbortController
-active_operation_identifier: string
-active_operation_type: string
+beginInstallOperation() AbortController
+cancelInstallOperation() void
+finishInstallOperation() void
+showAlertError(error: unknown) void
+update(extension: InstalledExtensionData, version: string) Promise~void~
+install(extension: InstalledExtensionData) Promise~void~
+finalizeExtension(extension: InstalledExtensionData) Promise~void~
+getTracker(signal: AbortSignal) PullTracker
+destroyed() void
}
class KrakenManager {
+installExtension(extension: InstalledExtensionData, progressHandler: function, signal: AbortSignal) Promise~void~
+updateExtensionToVersion(identifier: string, version: string, progressHandler: function, signal: AbortSignal) Promise~void~
+finalizeExtension(extension: InstalledExtensionData, tempTag: string, progressHandler: function, signal: AbortSignal) Promise~void~
+uninstallExtension(identifier: string) Promise~void~
+uninstallExtensionVersion(identifier: string, tag: string) Promise~void~
}
class PullProgress {
+download: number
+extraction: number
+statustext: string
+show: boolean
+cancelable: boolean
+$emit_cancel()
}
ExtensionManagerView --> KrakenManager : calls
ExtensionManagerView --> PullProgress : binds_props_and_listens_cancel
class StreamingUtils {
+generator_wrapper(gen: AsyncGenerator[str|bytes, None], queue: asyncio.Queue) AsyncGenerator[str|bytes, None]
+_fetch_stream(gen: AsyncGenerator[bytes, None], queue: asyncio.Queue) None
}
StreamingUtils ..> Extension : wraps_install_update_generators
ExtensionRouterV2 ..> StreamingUtils : uses_streamer_wrapper
ExtensionRouterV1 ..> StreamingUtils : uses_streamer_wrapper
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
uninstallExtensionVersionhelper callsDELETE /extension/${identifier}/${tag}, but there is no corresponding v2 route added inextension.py; if this endpoint doesn’t already exist elsewhere, this will consistently 404 and should either be implemented or the URL adjusted to an existing route. - In
finalizeExtensionUpload, theaxios.isCancelbranch always resetsinstall_from_file_phaseto'ready'and clearsinstall_from_file_status_textbefore checking whethercontroller === this.active_abort_controller; this can cause a stale controller’s cancellation to reset the UI during a new operation, so consider moving the UI reset inside thecontroller === this.active_abort_controllercheck. - The
PullProgressdialog is always rendered ascancelablefromExtensionManagerVieweven when there is no active abortable operation; tyingcancelableto!!active_abort_controllerwould prevent showing a cancel button that can’t actually affect any in-flight request.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `uninstallExtensionVersion` helper calls `DELETE /extension/${identifier}/${tag}`, but there is no corresponding v2 route added in `extension.py`; if this endpoint doesn’t already exist elsewhere, this will consistently 404 and should either be implemented or the URL adjusted to an existing route.
- In `finalizeExtensionUpload`, the `axios.isCancel` branch always resets `install_from_file_phase` to `'ready'` and clears `install_from_file_status_text` before checking whether `controller === this.active_abort_controller`; this can cause a stale controller’s cancellation to reset the UI during a new operation, so consider moving the UI reset inside the `controller === this.active_abort_controller` check.
- The `PullProgress` dialog is always rendered as `cancelable` from `ExtensionManagerView` even when there is no active abortable operation; tying `cancelable` to `!!active_abort_controller` would prevent showing a cancel button that can’t actually affect any in-flight request.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
nicoschmdt
force-pushed
the
stop-install
branch
from
8b9d9f7 to
453b3c1
Compare
joaomariolago
requested changes
Mar 27, 2026
Collaborator
There was a problem hiding this comment.
There is one edge case that seem to be happening with the PR that is the following:
- Have extension A V1 currently installed
- Go to extension store and open extension A popup
- Select extension A V2 and start the update
- Before the update download be complete, cancel it using the new
cancelbutton - Check Kraken settings file (Both extension A V1 and V2 are gone)
- Check docker images
docker image ls -a, extension A image is left there
Seems that this behavior breaks the atomic behavior of the Kraken install operation and could lead to images leaking on the docker side. Since we can't just delete images that are not associated with extensions given that it could be a user custom image used to other function.
nicoschmdt
force-pushed
the
stop-install
branch
from
453b3c1 to
9e04f46
Compare
Member
|
@nicoschmdt can you take a look in the comments and CI ? |
nicoschmdt
force-pushed
the
stop-install
branch
3 times, most recently
from
ec8b927 to
a757ee0
Compare
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
uninstallExtensionVersionfrontend helper targetsDELETE /extension/{identifier}/{tag}, but there is no corresponding v2 route added inapi/v2/routers/extension.py, so calls to this function will likely 404 until a backend handler is implemented. - In
ExtensionManagerView.vue,beginInstallOperationaborts any existingactive_abort_controllerbefore starting a new one, but the previous operation’sfinallyblocks only reset state when their controller matchesactive_abort_controller, which can leave stale installing state if a second operation is started while a first is still cleaning up; consider centralizing cleanup so aborted-but-superseded operations still clear their UI state.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `uninstallExtensionVersion` frontend helper targets `DELETE /extension/{identifier}/{tag}`, but there is no corresponding v2 route added in `api/v2/routers/extension.py`, so calls to this function will likely 404 until a backend handler is implemented.
- In `ExtensionManagerView.vue`, `beginInstallOperation` aborts any existing `active_abort_controller` before starting a new one, but the previous operation’s `finally` blocks only reset state when their controller matches `active_abort_controller`, which can leave stale installing state if a second operation is started while a first is still cleaning up; consider centralizing cleanup so aborted-but-superseded operations still clear their UI state.
## Individual Comments
### Comment 1
<location path="core/libs/commonwealth/src/commonwealth/utils/streaming.py" line_range="73-76" />
<code_context>
finally:
if heartbeat_task:
heartbeat_task.cancel()
+ await gen.aclose()
await queue.put(None)
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard `gen.aclose()` against errors so the sentinel is still queued and consumers can't hang.
If `gen.aclose()` raises (e.g. during generator finalization), it will skip `await queue.put(None)` and terminate the wrapper task without signaling completion, potentially leaving consumers blocked on `queue.get()`. Please wrap `gen.aclose()` in a try/except (with optional logging) so that `queue.put(None)` is reliably executed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix: #3631
since we intend to move away from FastAPI and into zenoh I focused the operation cancellation handling mostly on the frontend code.
Summary by Sourcery
Add support for cancelling in-progress Kraken extension install and update operations from the frontend while simplifying backend install/update behavior and streaming lifetime management.
New Features:
Bug Fixes:
Enhancements: